@philliphoff Any other changes you think should be made here?

I'm seeing the E2E tests fail on my system following the recent merge with master. In hopes those are due to something wrong with those tests, I'll hold off on any other changes to this PR to address them until it's confirmed it's a problem with this PR and not them.

I did add two more unit tests to validate the DaprClientBuilder a bit more where it was lacking existing tests and those are passing.

cc @halspang @philliphoff

@philliphoff I've thought about where you were coming from in the review and have overhauled the API substantially. There's now a single ScheduleJobAsync method that takes a DaprJobSchedule along with the other optional arguments. Each of the scheduling extension methods have been removed in favor of two that take an object and serialize it with the provided (albeit optional) JsonSerializerOptions and another that takes a string (presumably already serialized) and simply encodes it to UTF-8 bytes.

That leaves the GetJobsAsync and DeleteJobsAsync otherwise untouched.

A DaprJobSchedule supports the prefixed periods, Cron expressions, duration intervals and point-in-time date/time values via a series of factory methods:

• Yearly or Monthly (among others, except "@every") for prefixed "@Yearly" and "@monthly" expressions, respectively)
• FromDateTime accepts a DateTimeOffset with a specified point-in-time value and converts to an ISO8601 value
• FromDuration accepts a TimeSpan and converts to the Goland duration value
• FromExpression accepts an open-ended string expression (and the comment calls out that it accepts either a Cron-like or '@' prefixed period string - it doesn't do any validation except null/empty checking
• FromCronExpression is for those that want a more guided experience building Cron expressions. I added a CronExpressionBuilder class with a fluent API for building out various valid expressions (on T value, every U period, from V to W periods, on X,Y,Z days/months). It overloads ToString to output the resulting expression, but it can be passed directly into FromCronExpression if the user wants to take that approach over providing their own value to FromExpression.

The DaprJobSchedule class uses an internal constructor - it's expected the users will use the factory methods to build it (this was mostly to avoid just having umpteen constructor overloads, but if you think that approach is clearer over the factories, I can pivot).

Because a JobDetails now returns a DaprJobSchedule, the former also includes four boolean properties identifying the expression value as a prefixed period, point-in-time, interval or Cron expression and the value itself can be retrieved from ExpressionValue.

I removed the exceptions as they weren't actually used anymore and cleaned up the larger solution to remove the use of the two shared types in favor of accessing them from Dapr.Common. I've set up InternalsVisibleTo for each project in the solution in that project so that the types aren't exposed as part of the public API, so this PR now tweaks just about all projects in the solution in this regard.

I'm working to add more unit tests for the CronExpressionBuilder and make sure I didn't miss any edge situations.

I'm going to go back through the remaining review comments and address each, but I wanted to put the latest changes in one easy-to-find place.

In the last several commits, I've been working to build out a regular expression that can evaluate whether a value in a DaprJobSchedule is a Cron expression or not (so it can be a bit more reliable than "it's not any of the other types, so it must be Cron"). I've made incremental progress to capture each of the separate segments, add testing and address unsupported cases, but it passes the current test suite which is pretty comprehensive right now.

I did want to call out why I opted for the approach I did in the latest commit. I created a separate project to run some benchmarking because I wanted to implement this in a performant way that also limited allocations and the results were mildly surprising. I have three different approaches called SeparateRegex, CombinedRegex and GeneratorRegex and they differ as follows:

SeparateRegex

This approach reflects how I developed the whole expression - one segment at a time. It works by splitting the expression by a whitespace character. If there are fewer than 6 segments, it's not a Cron expression. Otherwise, evaluate each segment by its respective regular expression and return a boolean value reflecting if they all were a match or not.

CombinedRegex

This approach builds off the completed work of the last one in that I create a compiled regular expression of the entire string (as follows) and evaluate the expression as provided:

([0-5]?\d-[0-5]?\d)|([0-5]?\d,?)|(*(/[0-5]?\d)?) ([0-5]?\d-[0-5]?\d)|([0-5]?\d,?)|(*(/[0-5]?\d)?) (([0-1]?\d)|(2[0-3])-([0-1]?\d)|(2[0-3]))|(([0-1]?\d)|(2[0-3]),?)|(*(/([0-1]?\d)|(2[0-3]))?) *|(*/(([0-2]?\d)|(3[0-1])))|(((([0-2]?\d)|(3[0-1]))(-(([0-2]?\d)|(3[0-1])))?)) (^(*/)?((0?\d)|(1[0-2]))$)|(^*$)|(^((0?\d)|(1[0-2]))(-((0?\d)|(1[0-2]))?)$)|(^(?:JAN|FEB|MAR|APR|MAY|JUN|JUL|AUG|SEP|OCT|NOV|DEC)(?:-(?:JAN|FEB|MAR|APR|MAY|JUN|JUL|AUG|SEP|OCT|NOV|DEC))?(?:,(?:JAN|FEB|MAR|APR|MAY|JUN|JUL|AUG|SEP|OCT|NOV|DEC)(?:-(?:JAN|FEB|MAR|APR|MAY|JUN|JUL|AUG|SEP|OCT|NOV|DEC))?)*$) *|(*/(0?[0-6])|(0?0-6?)|((,?(SUN|MON|TUE|WED|THU|FRI|SAT))+)|((SUN|MON|TUE|WED|THU|FRI|SAT)(-(SUN|MON|TUE|WED|THU|FRI|SAT))?))

GeneratorRegex

This utilizes the new approach introduced in .NET 7 of using a source generator to produce a pared down version of the regular expression engine to only those bits necessary for the provided expression.

The results of a standard benchmarking run + memory diagnostics are as follows:

Surprisingly, Separate performed the best, followed by the Generator approach and then Combined (all within 2 microseconds of one another, so pretty similar). But the allocations were a distinctly different story: Separate performed the worst by far with 5616 bytes versus 216 bytes for Combined and 152 for the Generator, which make a lot of sense and align with expectations.

As the 1.15 release of Dapr will precede the release of .NET 9 and will continue to have full support for .NET 6, it means that while Regex source generators were introduced with .NET 7, use of the GeneratedRegexAttribute itself requires .NET 7 and and later so it's not yet available for all targeted versions (Dapr targets the maximum common language version of C# 10 right now) so in a future release, we can make this tweak and save a few more kb of allocations.

Codecov Report

Attention: Patch coverage is 77.77778% with 84 lines in your changes missing coverage. Please review.

Project coverage is 68.01%. Comparing base (1b7c9f4) to head (d6e88d2).
Report is 19 commits behind head on master.

@@            Coverage Diff             @@
##           master    #1331      +/-   ##
==========================================
+ Coverage   67.28%   68.01%   +0.72%     
==========================================
  Files         174      187      +13     
  Lines        6025     6352     +327     
  Branches      671      722      +51     
==========================================
+ Hits         4054     4320     +266     
- Misses       1802     1853      +51     
- Partials      169      179      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.